Skip to content

Comments

missing_field_behavior 's default to ignore#56

Merged
hiraokashi merged 11 commits intomasterfrom
hiraokashi/change_missing_field_behavior_default
Nov 12, 2025
Merged

missing_field_behavior 's default to ignore#56
hiraokashi merged 11 commits intomasterfrom
hiraokashi/change_missing_field_behavior_default

Conversation

@hiraokashi
Copy link
Contributor

@hiraokashi hiraokashi commented Sep 11, 2025

Why

https://github.com/wantedly/backend-chapter/issues/174

What

  • pb-serializer の missing_field_behavior のデフォルトを ignore に変更した
  • CIのruby versionを3.0以降に変更
  • それに伴い、simplecov-coberturaを最新化

it { is_expected.to be_a TestFixture::Simple::Message }
end

context 'when missing_field_behavior is not configured (default behavior)' do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[rubocop] reported by reviewdog 🐶
Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

# Override the parent's before block to avoid setting missing_field_behavior
let(:missing_field_behavior) { nil }

it 'uses default value :ignore' do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[rubocop] reported by reviewdog 🐶
Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

spec.add_development_dependency "sqlite3", "~> 1.4"
spec.add_development_dependency "simplecov", "~> 0.18.5"
spec.add_development_dependency "simplecov-cobertura", "~> 1.3"
spec.add_development_dependency "concurrent-ruby", "1.3.4"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[rubocop] reported by reviewdog 🐶
Gemspec/OrderedDependencies: Dependencies should be sorted in an alphabetical order within their section of the gemspec. Dependency concurrent-ruby should appear before simplecov-cobertura.

# ruby: [ 2.5, 2.6, 2.7, 3.0 ]
ruby: [ 2.5, 2.6, 2.7 ]
exclude:
- { os: macos-latest, ruby: '2.5' }
Copy link
Contributor Author

@hiraokashi hiraokashi Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

macos-latestの2.5はもうサポートされていなかったので、
excludeを使ってみようとすると、Expected — Waiting for status to be reported なってしまいCIが
終わらない。

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Branch Protection Rule で、Status checks that are required からも消す必要があります(わかりづらいですよね... 自分も前にハマりました)

https://github.com/wantedly/pb-serializer/settings/branch_protection_rules/15047227

CleanShot 2025-09-11 at 17 37 53@2x

Copy link
Member

@stomk stomk Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI で動かしている Ruby のバージョン、[ 2.5, 2.6, 2.7 ] だと古すぎるので、3.0 以降とかにしてしまっていいと思いますね。
gemspec にある Rails (activerecord)のバージョン指定が、rails_versions = [">= 5.2", "< 6.1"] になっているので、Rails バージョンも一緒に上げてしまいたいですね。

差分多くなりそうなので、別PRでやりましょう。

spec.add_development_dependency "sqlite3", "~> 1.4"
spec.add_development_dependency "simplecov", "~> 0.18.5"
spec.add_development_dependency "simplecov-cobertura", "~> 1.3"
spec.add_development_dependency "simplecov", "~> 0.19"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[rubocop] reported by reviewdog 🐶
Gemspec/OrderedDependencies: Dependencies should be sorted in an alphabetical order within their section of the gemspec. Dependency simplecov should appear before sqlite3.

@hiraokashi hiraokashi requested a review from gedorinku October 9, 2025 02:59
Copy link
Member

@gedorinku gedorinku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hiraokashi hiraokashi merged commit 8ab852c into master Nov 12, 2025
9 checks passed
@hiraokashi hiraokashi deleted the hiraokashi/change_missing_field_behavior_default branch November 12, 2025 23:23
@hiraokashi hiraokashi mentioned this pull request Nov 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants